fix(sdk/go): use the CLI oidc_token.json format - #4830

cc-review Summary

What Went Well

• Genuine cross-language anti-drift: the single fixture sdk/conformance/oidc-token-bundle.json is asserted from both Rust (oidc_token.rs:197) and Go (read oidc/token_test.go:28, write :43), so a field change on either side breaks that language's test.
• internal/oidctoken centralizes the on-disk Bundle type and converters, removing the two divergent private structs (the root-cause fix for #4508).
• Missing/null expires_at is handled consistently as non-expiring on every path (Go and Rust), with dedicated nil-vs-zero tests.
• The RefreshableToken backoff path is nil-error safe: nextRetry/lastErr are set together on failure and cleared together on success.
• Go token persistence is atomic and 0600 (CreateTemp + Chmod + Sync + Rename) with temp-file cleanup on every error path.

Findings

Two findings are summary-only because they fall on lines outside this PR's diff:

• crates/openshell-bootstrap/src/oidc_token.rs:54 (Minor, pre-existing store_oidc_token): non-atomic std::fs::write + separate chmod 0600 leaves a brief wider-permission window and a truncating in-place write that the Go reader can observe mid-write. The Go writer this PR adds does it correctly. In the issue's stated scope; mitigated by the 0700 parent dir and the cached-token fallback.
• sdk/go/openshell/v1/auth_refresh.go:82 (Notable): a no-expiry cached token makes isTokenValid() always true, so a long-lived process never re-reads the file and won't pick up a later CLI-written expiring token until restart. Consistent with the Rust side; the only defensible behavior without expiry metadata.

Review Details

• Findings posted: 6 (4 inline, 2 summary-only)
• Gate outcome: PASS (0 Critical, 0 Important)
• Participating reviewers: goal-alignment, production, security, gator (architecture, correctness, test-quality ran with no surviving findings; codex and coderabbit skipped on this fork PR)

Here's is another round of reviews on the latest changes (with updated agents, so there might be findings that the first round hasn't found). Although there are some findings (hopefully easy to fix by aganets), I think we are close to converge.

cc-review Summary

New round with fresh agent eyes on 6054443. The earlier threads are either addressed or continue in their existing threads (including the open issuer/client_id matching question), so this round lists only new observations. The findings come from an agent-assisted review; the failure scenarios were derived from the code and not executed.

What Went Well

• sdk/go/openshell/v1/internal/oidctoken/oidctoken.go: one internal Bundle type with FromToken/Token replaces the two divergent oidcBundle copies in gateway/token.go and oidc/token.go. Its field tags (omitempty on refresh_token, *uint64 expires_at, required issuer/client_id) mirror the serde attributes of OidcTokenBundle in openshell-bootstrap.
• sdk/conformance/oidc-token-bundle.json is checked from both sides: Rust conformance_fixture_matches_cli_serialization (crates/openshell-bootstrap/src/oidc_token.rs) and Go TestWriteToken_MatchesConformanceFixture and TestReadToken_ConformanceFixture, so a format drift between CLI and SDK fails CI.
• sdk/go/openshell/v1/gateway/token.go diskTokenSource: bundles without issuer/client_id are rejected instead of being treated as never-expiring, the expiry check fails closed at the exact expiry instant, and loadError gives every failure branch the ErrTokenLoad wrap and the openshell gateway login <name> hint without putting token material in the message.
• sdk/go/openshell/v1/auth_refresh.go usableCached/lastErr: after a failed refresh, an expired cached bearer is no longer sent, and the backoff path returns the real refresh error. lastErr is read and written under r.mu and cleared on success. TestGetRequestMetadata_RefreshFailureWithExpiredCachedTokenReturnsError covers it with require.ErrorIs, a check that no "using cached token" log was emitted, and a source call count.
• sdk/go/openshell/v1/oidc/oidc.go Login reuses a stored token only when the normalized issuer and the client ID match, and TestLogin_ReplacesMismatchedStoredToken covers the client-id, issuer, and legacy-format cases in one table.
• TestDiskTokenSource_ExpiredToken and TestDiskTokenSource_NoExpiry table-test the boundary values expires_at=0, an absent field, and explicit null.

Findings

Not Posted Inline

sdk/go/docs/src/api/gateway.md:160: Location is outside the PR diff (line 160).

Minor: The ErrTokenLoad row in the error table still says "Token file missing or unreadable", and so does gateway/doc.go:64. The new paragraph right above the table (lines 151-154) lists a missing, malformed, or expired oidc_token.json, and the comment in gateway/errors.go:19-20 says "missing, unreadable, or malformed". None of these reference spots mention expired tokens.

Why this matters: The table introduces errors.Is classification, so a reader could wrap an RPC in errors.Is(err, gateway.ErrTokenLoad) to catch an expired OIDC token. That check never matches: the sentinel comes from GetRequestMetadata, gRPC turns it into a status, and the SDK StatusError does not keep the wrap chain. The paragraph above already points to v1.IsUnauthenticated, so this is about keeping the reference row in line with it. Expired tokens returned ErrTokenLoad before this PR too, so the wording gap is older than this change.

Suggested fix: In the table and in doc.go, either drop ErrTokenLoad or describe it as "returned by the auth provider's GetRequestMetadata for missing, malformed, expired, or pre-CLI-format token files; on RPCs it surfaces as Unauthenticated". Add the expired case to the errors.go comment.

Source: architecture agent

Review Details

• Findings posted: 6
• Findings reviewed and not posted: 1
• Gate outcome: PASS
• Participating reviewers: architecture, gator, production, test-quality

Reviewed CLI token format compatibility, cached-login validation, expiry handling, and refresh failure backoff. No actionable findings. The affected Go packages passed with the race detector, and required CI checks are passing.